Skip to content

Remove bulk_create due to changed behavior between dj32 and dj40. #1171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 14, 2022
Merged

Remove bulk_create due to changed behavior between dj32 and dj40. #1171

merged 1 commit into from
Jun 14, 2022

Conversation

n2ygk
Copy link
Contributor

@n2ygk n2ygk commented Jun 3, 2022

Fixes #1165

Description of the Change

Removes problematic bulk_create from test_models.

  • Fixes erroneous test that was missing cases under dj32 because pk was being returned as null and not noticed.
  • Resolves errors running under dj40 (albeit only locally on MacOS -- for which I still don't understand why).
  • Updates tests to confirm that test objects were in fact created as expected.

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

@n2ygk n2ygk added this to the 2.1.0 milestone Jun 3, 2022
@n2ygk n2ygk requested a review from a team June 3, 2022 16:05
@codecov
Copy link

codecov bot commented Jun 3, 2022 •
edited
Loading

Codecov Report

Merging #1171 (71b3002) into master (f4136bf) will decrease coverage by 0.05%.
The diff coverage is n/a.

❗ Current head 71b3002 differs from pull request most recent head bd07ad4. Consider uploading reports for the commit bd07ad4 to get more accurate results

@@            Coverage Diff             @@
##           master    #1171      +/-   ##
==========================================
- Coverage   96.90%   96.85%   -0.06%     
==========================================
  Files          31       31              
  Lines        1812     1812              
==========================================
- Hits         1756     1755       -1     
- Misses         56       57       +1
Impacted Files Coverage Δ
oauth2_provider/models.py 98.52% <0.00%> (-0.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4136bf...bd07ad4. Read the comment docs.

@n2ygk n2ygk force-pushed the remove_bulk_create branch 2 times, most recently from a761b04 to e650606 Compare June 3, 2022 18:17
@n2ygk n2ygk merged commit a12a56e into django-oauth:master Jun 14, 2022
@n2ygk n2ygk deleted the remove_bulk_create branch June 14, 2022 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@Andrew-Chen-Wang Andrew-Chen-Wang Andrew-Chen-Wang approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.

[maintainers] why is test_models.py throwing an error for dj40 locally but not in GH actions?
2 participants
@n2ygk @Andrew-Chen-Wang